Storing and tracking the onboarding messaging use case #5009

ouchadam · 2022-01-20T11:17:34Z

Part of #4585

Adds storing and tracking of the selected messaging use case as part of the onboarding journey (this will eventually be used to better tailor the first time experience)
Adds VectorAnalytics.updateUserProperties to allow for updating the identity/user properties outside of the analytics id/consent flow, eg locally capturing the onboarding selection and only sending after the user has consented.Posthog.Identify() accumulates a local cache which is only sent once a user has consented Posthog.OptOut(false)

ouchadam · 2022-01-20T11:32:28Z

marked as draft whilst confirming if the identity payload is coming through

github-actions · 2022-01-20T12:00:38Z

Unit Test Results

  72 files ±0   72 suites ±0 1m 1s ⏱️ ±0s
141 tests ±0 141 ✔️ ±0 0 💤 ±0 0 ❌ ±0
440 runs ±0 440 ✔️ ±0 0 💤 ±0 0 ❌ ±0

Results for commit cbdeb54. ± Comparison against base commit 38a6c3e.

♻️ This comment has been updated with latest results.

github-actions · 2022-01-20T13:57:38Z

Matrix SDK

Integration Tests Results:

[org.matrix.android.sdk.session]
passed="21" failures="0" errors="0" skipped="2"
[org.matrix.android.sdk.account]
passed="5" failures="0" errors="0" skipped="2"
[org.matrix.android.sdk.internal]
passed="158" failures="1" errors="0" skipped="38"
[org.matrix.android.sdk.ordering]
passed="16" failures="0" errors="0" skipped="0"
[org.matrix.android.sdk.PermalinkParserTest]
passed="2" failures="0" errors="0" skipped="0"

bmarty

LGTM, some remarks

bmarty · 2022-01-20T15:42:19Z

vector/src/main/java/im/vector/app/features/onboarding/FtueUseCase.kt

+    }
+}
+
+fun FtueUseCase.toTrackingValue(): Identity.FtueUseCaseSelection {


I put the extensions about analytics in the file https://github.com/vector-im/element-android/blob/a8c29f55f5a65cbf404dfe710cd9a6a3e976b6da/vector/src/main/java/im/vector/app/features/analytics/extensions/JoinedRoomExt.kt#L24, but not merge, it's part of #4734.

will do the same (in a separate FtueUseCaseExt.kt file) 👍

bmarty · 2022-01-20T15:45:09Z

vector/src/main/java/im/vector/app/features/onboarding/store/OnboardingStore.kt

+import kotlinx.coroutines.flow.first
+import javax.inject.Inject
+
+private val Context.dataStore: DataStore<Preferences> by preferencesDataStore(name = "vector_onboarding")


I wonder how we will manage mutli accounts, if we manage it one day.
We should probably have scoped pref: global / per session.
Just thinking out loud.

that's a good point, delaying the storing until after sign up makes sense to me, then we can associate to a session/account

I'm happy to try converting this to a realm entity in the global database (by user)

assuming we're happy to avoid using a database for the time being~ I've updated the onboarding store to be by user id efbdd70

ouchadam · 2022-01-21T12:18:05Z

vector/src/main/java/im/vector/app/features/analytics/VectorAnalytics.kt

+    /**
+     * Update user specific properties
+     */
+    fun updateUserProperties(identity: Identity)


@bmarty I'll also move this to the AnalyticsTracker

bmarty · 2022-01-24T12:10:22Z

vector/src/main/java/im/vector/app/features/onboarding/store/OnboardingStore.kt

+class OnboardingStore @Inject constructor(
+        private val context: Context
+) {
+    suspend fun readUseCase(userId: String) = context.dataStore.data.first().let { preferences ->


Q: is it possible to have a OnBoardingStore scoped to the session to avoid passing the userId to each of its fun?

we could do it for the MainActivity injection site (for cleaning up the local storage) but not the OnboardingViewModel as the session doesn't exist yet

alternatively we could have a provider/factory as a session extension instead of injecting the dependency, what do you think?

Session.onboardingStore(context: Context) = OnboardingStore(context, myUserId)

this looks neat!

bmarty · 2022-01-25T22:32:16Z

vector/src/main/java/im/vector/app/features/onboarding/store/OnboardingStore.kt

+    }
+
+    private fun String.toUseCaseKey() = stringPreferencesKey("$this-use_case")
+}


This class could be simplified with:

class OnboardingStore( private val context: Context, myUserId: String ) { private val useCasePreferences = stringPreferencesKey("$myUserId-use_case") suspend fun readUseCase() = context.dataStore.data.first().let { preferences -> preferences[useCasePreferences]?.let { FtueUseCase.from(it) } } suspend fun setUseCase(useCase: FtueUseCase) { context.dataStore.edit { settings -> settings[useCasePreferences] = useCase.persistableValue } } suspend fun resetUseCase() { context.dataStore.edit { settings -> settings.remove(useCasePreferences) } } suspend fun clear() { resetUseCase() } }

and removeKeysWithPrefix is not necessary since we know have a new per userId store object.

I am also wondering if we could make it more generic, so its goal will be to store any data app side related to the session. I have in mind the storage of a client secret for matrix-org/sygnal#70.

The class name could be something like SessionDataStore (open for better ideas!)

sounds good to me 👍

went with VectorSessionStore 64c18ac

…r analytics

…arding flow

…or the smaller tracking subset

- this ensures we have a unique session or account id to store the selection against in case we support multi account in the future

- will enable multi account support in the future

- avoids needing to know about the user id for each read/write

bmarty · 2022-01-26T12:42:00Z

vector/src/main/java/im/vector/app/features/session/VectorSessionStore.kt

+        myUserId: String
 ) {

+    private val Context.dataStore: DataStore<Preferences> by preferencesDataStore(name = "vector_session_store_$myUserId")


I am wondering if directly using the userId may lead to some trouble: special chars - low risk, length - higher risk. Maybe using a hash of it would be better.

very good point, I keep forgetting that user-id is actually a matrix id rather than a uuid, will change

Yes. "Matrix Id" is not really a specific concept, but it is sometimes used for user ids. https://spec.matrix.org/v1.1/appendices/#identifier-grammar is an interesting reading on the subject

bmarty

LGTM, thanks for all the updates!

ouchadam added Z-FTUE Issue is relevant to the first time use project or experience Z-Tracking labels Jan 20, 2022

ouchadam mentioned this pull request Jan 20, 2022

Build FTUE Use Case screen #4585

Closed

6 tasks

bmarty approved these changes Jan 20, 2022

View reviewed changes

ouchadam commented Jan 21, 2022

View reviewed changes

ouchadam force-pushed the feature/adm/storing-use-case branch from 0222a33 to efbdd70 Compare January 21, 2022 13:50

ouchadam marked this pull request as ready for review January 21, 2022 14:25

bmarty reviewed Jan 24, 2022

View reviewed changes

ouchadam force-pushed the feature/adm/storing-use-case branch from ff109c0 to bcb065a Compare January 25, 2022 17:54

bmarty reviewed Jan 25, 2022

View reviewed changes

ouchadam force-pushed the feature/adm/storing-use-case branch from 42f5ba8 to 64c18ac Compare January 26, 2022 09:18

ouchadam added 9 commits January 26, 2022 09:35

storing the user usecase selection and clearinng on signout

84e23e1

adding ability to update the user properties (identity) via the vecto…

515c8ce

…r analytics

updating the identity with the use case selection as part of the onbo…

6c1cbcc

…arding flow

making use of the analytics tracker instead of the vector analytics f…

0a7c421

…or the smaller tracking subset

persisting the use case at the point of session creation

c4ac039

- this ensures we have a unique session or account id to store the selection against in case we support multi account in the future

storing an retrieving the use case selection by user

f44ccd7

- will enable multi account support in the future

lifting analytics extension to the common package

b91b9cb

providing the onboarding store via session extension

73b80b1

- avoids needing to know about the user id for each read/write

making the onboarding store a userId scoped generic store instead

bc37391

ouchadam force-pushed the feature/adm/storing-use-case branch from 64c18ac to bc37391 Compare January 26, 2022 09:35

bmarty reviewed Jan 26, 2022

View reviewed changes

using md5 hash of the userId to normalise the usage as a key

cbdeb54

ouchadam requested a review from bmarty January 27, 2022 12:14

bmarty approved these changes Jan 27, 2022

View reviewed changes

ouchadam merged commit 986d9f9 into develop Jan 31, 2022

ouchadam deleted the feature/adm/storing-use-case branch January 31, 2022 14:15

Storing and tracking the onboarding messaging use case #5009

Storing and tracking the onboarding messaging use case #5009

Uh oh!

Conversation

ouchadam commented Jan 20, 2022

Uh oh!

ouchadam commented Jan 20, 2022

Uh oh!

github-actions bot commented Jan 20, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Unit Test Results

Uh oh!

github-actions bot commented Jan 20, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Matrix SDK

Integration Tests Results:

Uh oh!

bmarty left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ouchadam Jan 26, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

bmarty left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

github-actions bot commented Jan 20, 2022 •

edited

Loading

github-actions bot commented Jan 20, 2022 •

edited

Loading

ouchadam Jan 26, 2022 •

edited

Loading